Conversation
Find Lexical Entry now matches typed characters anywhere in a word for queries of 3+ characters, using liblcm's new SearchType.Substring; shorter queries keep the original full-text search. The mode decision lives in a unit-tested SubstringSearchPolicy, and results retain the existing exact -> starts-with -> anywhere ordering.
Comment hygiene (advisory)No comment-style violations in the lines this branch adds since |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1069 +/- ##
==========================================
- Coverage 38.35% 38.35% -0.01%
==========================================
Files 1507 1508 +1
Lines 350617 350641 +24
Branches 40298 40301 +3
==========================================
+ Hits 134471 134480 +9
- Misses 186916 186931 +15
Partials 29230 29230
🚀 New features to boost your workflow:
|
ResetMatches now swaps the matching browser's search engine on each query to move between full-text and substring modes. That swap always installed an EntryGoDlg-owned engine, replacing the MergeEntryDlg engine that excludes the starting entry. As soon as the Merge Entry dialog set its form text (or the user typed), the starting entry reappeared in the list and could be merged into itself. Make the full-text and substring engine accessors virtual on EntryGoDlg so MergeEntryDlg can supply engines that exclude the starting entry in both modes, and drop MergeEntryDlg's InitializeMatchingObjects override now that the base engine selection handles it. Add tests covering the exclusion in full-text and substring modes. Co-Authored-By: Claude <noreply@anthropic.com>
jasonleenaylor
left a comment
There was a problem hiding this comment.
Two things I'd like changed.
UseSubstringcounts UTF-16 units, not characters.string.LengthafterFormCis still code units, and the comment says a base character plus a combining diacritic counts as one. That only holds when the pair has a precomposed form. Calling your ownUseSubstringon the built branch:
two user-perceived characters each FormC.Length UseSubstring
e+U+0301 then o (acute composes) 2 false
b+U+0303 then o (tilde, no precomposed) 3 TRUE
n+U+0330 then o (tilde below) 3 TRUE
t+U+032C then o (caron below) 3 TRUE
U+10480 then o (non-BMP, surrogate pair) 3 TRUE
Four of those five two-character queries turn substring matching on, and the only one that doesn't is the one whose diacritic happens to compose. So the threshold fires a character early in exactly the orthographies this feature is most useful for, which is also where your rationale about matching most entries in a large project bites hardest.
UseSubstring_countsComposedCharacters_notUtf16Units doesn't catch it because its fixture is ComposedAcuteE repeated, and acute is one of the compositions that exists. The test passes and pins a belief that doesn't generalize. StringInfo.LengthInTextElements gives 1 for every single grapheme above and 2 for every two-grapheme query, so it's a one-line change that makes the code do what the comment already promises. Whichever way you go, the test needs a non-composable case.
- The substring engine never gets the warm-up the full-text engine gets.
InitializeMatchingObjectsends withSearchAsync(GetFields(string.Empty, ...))under a "start building index" comment, and that reaches whichever engine the browser holds, which at that point is the full-text one becauseSearchEngineFor(string.Empty)returns it. So the substring engine's index starts at zero, and a cold engine delivers nothing until its index is complete:PerformSearchbuilds the index before it searches, and a search the next keystroke cancels returns null with no event, so results only ever arrive from a search that finished indexing.GetSearchableObjectsisILexEntryRepository.AllInstances().ToArray(), every entry in the project. So crossing into substring matching, which is where the user expects results to get better, is where the first results have to wait on a full index build.
Simplest fix I can see: alongside the existing warm-up, call SubstringSearchEngine.SearchAsync(GetFields(string.Empty, selectedWs.Handle)). SearchAsync is public, and nothing is subscribed to that engine's SearchCompleted until SetSearchEngine hooks it, so the completion is a no-op. It has to come after Initialize because GetFields reads IsVisibleColumn, which is where the existing call already sits. Since SearchEngine.Get caches in the property table, that's one extra background build per session rather than per dialog open. A more surgical option is to warm it when the key reaches MinQueryLength - 1, so the build overlaps the keystroke before it's needed.
Either way, please say in the body that this keeps two indexes over the lexicon. Two StringSearcher instances, two entry arrays, two ConsumerThreads and two IVwNotifyChange registrations, so every data change now notifies both. I think that's the right trade against rebuilding the index on every mode flip, but it should be a decision on the record rather than a side effect.
Worth keeping as it is: SearchEngineFor gets the key after FindMorphType has stripped reserved characters, SetSearchEngine unhooks the old SearchCompleted before rehooking, and moving CurrentEntryHvo onto a property refreshes it per access instead of once at init.
Built and ran the branch at ff11df9: build.ps1 clean, and all ten of your new tests pass. The table above is from invoking the compiled SubstringSearchPolicy.UseSubstring, not a standalone normalization check.
Quick Summary
Find Lexical Entry now matches typed characters anywhere in a word for queries of 3+ characters, using liblcm's new SearchType.Substring; shorter queries keep the original full-text search. The mode decision lives in a unit-tested SubstringSearchPolicy, and results retain the existing exact -> starts-with -> anywhere ordering.
Depends on sillsdev/liblcm#395
https://jira.sil.org/browse/LT-22524
CI-ready checklist
.github/commit-guidelines.md(subject ≤ 72 chars, no trailing punctuation; if body present, blank line then ≤ 80-char lines).Docs/workflows/ai-pr-workflow.mdand ranpr-preflightor the equivalent branch-readiness review before requesting review.Src/**folders touched, correspondingAGENTS.mdfiles are updated or explicitly confirmed still accurate.Notes for reviewers (optional)
This change is